[17.0][OU-ADD] hr_holidays: Migration to 17.0 #4702
Conversation
|
/ocabot migration hr_holidays |
openupgrade_scripts/scripts/hr_holidays/17.0.1.6/pre-migration.py
Outdated
Show resolved
Hide resolved
openupgrade_scripts/scripts/hr_holidays/17.0.1.6/pre-migration.py
Outdated
Show resolved
Hide resolved
openupgrade_scripts/scripts/hr_holidays/17.0.1.6/post-migration.py
Outdated
Show resolved
Hide resolved
remi-filament
force-pushed
the
17.0-hr_holidays
branch
from
8d3005b to
3749ab5
Compare
|
Done as per your comments, thanks for your review @Murtaza-SerpentCS |
carlos-lopez-tecnativa
left a comment
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
@remi-filament could you please rebase the branch?
openupgrade_scripts/scripts/hr_holidays/17.0.1.6/upgrade_analysis_work.txt
Show resolved
Hide resolved
remi-filament
force-pushed
the
17.0-hr_holidays
branch
from
3749ab5 to
e3b87ac
Compare
|
@carlos-lopez-tecnativa @MiquelRForgeFlow thanks for your review, rebase done and apriori updated. |
carlos-lopez-tecnativa
left a comment
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
Tested and working, but I think we need to adjust some SQL to maintain the current behavior in V16. Please let me know if you have any doubts.
TT49870
cc: @pedrobaeza
openupgrade_scripts/scripts/hr_holidays/17.0.1.6/upgrade_analysis_work.txt
Show resolved
Hide resolved
openupgrade_scripts/scripts/hr_holidays/17.0.1.6/pre-migration.py
Outdated
Show resolved
Hide resolved
|
ping @remi-filament |
remi-filament
force-pushed
the
17.0-hr_holidays
branch
2 times, most recently
from
f1607c3 to
3987e05
Compare
remi-filament
force-pushed
the
17.0-hr_holidays
branch
from
3987e05 to
882bc85
Compare
|
Thanks @carlos-lopez-tecnativa and @MiquelRForgeFlow I implemented the changes as per your review ! |
carlos-lopez-tecnativa
left a comment
carlos-lopez-tecnativa
left a comment
There was a problem hiding this comment.
Just a minor change, and I think it's ready to merge.
openupgrade_scripts/scripts/hr_holidays/17.0.1.6/pre-migration.py
Outdated
Show resolved
Hide resolved
remi-filament
force-pushed
the
17.0-hr_holidays
branch
from
882bc85 to
9776f3c
Compare
| hr_holidays / hr.leave.accrual.plan / active (boolean) : NEW hasdefault: default | ||
| # NOTHING TO DO: new field, default = active |
There was a problem hiding this comment.
please, do like _pre_create_account_report_active in pre-migration of account.
There was a problem hiding this comment.
Done, what is the rationale to create columns in SQL instead of letting Odoo do it, is it so much performance gain ?
Should we do this for all new fields with default value ? (there are lots of them on hr.leave.accrual.plan : accrued_gain_time, added_value_type, carryover_date, carryover_day, carryover_month).
There was a problem hiding this comment.
My logic is that if a model is transactional (with many records, such as account.move, account.move.line, stock.move, stock.move.line, sale.order, sale.order.line, etc.), it should be handled using SQL instead of the ORM. However, if the model is a master record (with only a few records, typically settings), then using the ORM is sufficient.
It would be great to hear the logic from others, such as @pedrobaeza @MiquelRForgeFlow, to take different perspectives into account.
remi-filament
force-pushed
the
17.0-hr_holidays
branch
from
9776f3c to
e017d85
Compare
|
/ocabot merge nobump |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at 9b26ece. Thanks a lot for contributing to OCA. ❤️ |
6 participants
No description provided.